Finalize terminal PR orchestration in the single-job flow - #501
Finalize terminal PR orchestration in the single-job flow#501sepo-agent-app[bot] wants to merge 1 commit into
Conversation
Rubrics Review
Notes
Findings
Final Rubric VerdictPARTIAL
|
![sepo-agent-app[bot]](https://avatars.githubusercontent.com/in/3527007?s=60&v=4){kind=small}
| AGENT_COLLAPSE_OLD_REVIEWS: ${{ vars.AGENT_COLLAPSE_OLD_REVIEWS }} | ||
| AGENT_HANDLE: ${{ vars.AGENT_HANDLE || '@sepo-agent' }} | ||
| AGENT_PROGRESS_COMMENT_ID: ${{ steps.planner.outputs.progress_comment_id }} | ||
| AGENT_PROGRESS_FINAL_COMMENT_MODE: merge |
There was a problem hiding this comment.
The progress reporter creates this comment with the job-scoped github.token, but this step runs with the resolved App/PAT as GH_TOKEN, and the CLI calls tryMergeProgressFinalComment without an explicit token. Pass AGENT_PROGRESS_GITHUB_TOKEN: ${{ github.token }} through to the merger; otherwise App-backed final edits can retrigger Agent Entrypoint, violating the non-recursive final-replacement contract merged in #500. Please add a token-sensitive merge/fallback regression.
| ) { | ||
| return false; | ||
| } | ||
| if (automationMode !== "agent") return true; |
There was a problem hiding this comment.
Heuristics mode has no planner user_message, yet this branch treats every exact source outcome as cleanup-eligible. The final note then contains only generic action/conclusion metadata while cleanup can minimize the detailed source synthesis. Keep publishing the terminal note, but fail closed on cleanup when no substantive cumulative summary is available, and cover that heuristics path with a regression.
| try { | ||
| const finalCommentId = currentFinalCommentId(prNumber, knownCommentId); | ||
| if (!finalCommentId) return; | ||
| const collapsed = collapsePreviousPrConversationArtifacts({ |
There was a problem hiding this comment.
This cleanup pass authenticates only as the ambient App/PAT, while the helper limits matches to comments authored by its GraphQL viewer. After #500, workflow-finalized fix/progress artifacts are github-actions[bot]-authored and this pass cannot collapse them. Please add an independently caught workflow-token cleanup pass, preserving the same cutoff and human/unrelated-bot exclusions, with cross-identity coverage.
AI Review Synthesis
Summary of PR/IssuePR #501 implements the lean redo requested by issue #496. It finalizes terminal PR orchestration in the current progress comment or a marker-based fallback, includes cumulative summaries and human requester mentions, and collapses older trusted artifacts after successful outcomes. The change preserves the single-job architecture and existing permissions across seven focused files. Documentation and regression coverage are updated without release-note changes. ReviewThe architecture and scope are appropriate, and the hosted Test Scripts check passes. However, current head
Progress
Issue DetailsProgress-comment replacement uses the wrong token identityCause: The workflow passes the progress comment ID but not Candidate solutions: Pass Comments: This is the same non-recursive final-replacement invariant established by merged PR #500. Cleanup misses workflow-authored artifactsCause: Terminal cleanup runs only with ambient App/PAT authentication, while the helper accepts only comments authored by its authenticated viewer. Candidate solutions: Retain the App/PAT pass and add an independently caught workflow-token pass when the tokens differ, preserving the current cutoff and human/unrelated-bot exclusions. Add cross-identity regression coverage. Heuristics cleanup can remove the only substantive summaryCause: Success validation accepts heuristics outcomes without a summary, while the final formatter and cleanup can replace the detailed source synthesis with generic metadata. Candidate solutions: Continue publishing the terminal note, but make cleanup fail closed when no substantive cumulative summary exists. Add a heuristics-mode regression without expanding summary provenance. Comments: This preserves the lean scope and avoids reintroducing the broader machinery rejected in earlier attempts. Reused fallback-note cutoff can leave newer artifacts visibleCause: Cleanup compares artifact database IDs against the reused final comment’s unchanged ID, so artifacts created after that old comment remain above the cutoff. Candidate solutions: In a follow-up, use an update-aware ordering boundary or explicitly exclude the current final node while retaining fail-closed protections. Comments: The rubric reviewer rated this a warning, while Claude treated it as INFO. Because cleanup is explicitly best-effort and the failure mode leaves comments visible, this synthesis classifies it as Recommended Next Step
Final Verdict
Action Items
|
1 participant
Summary
sepo-agent-orchestrate-stopfallbackVerification
npm --prefix .agent run buildnode --test .agent/dist/__tests__/orchestrate-handoff-cli.test.js .agent/dist/__tests__/review-summary-minimize.test.jsenv -u GITHUB_OUTPUT npm --prefix .agent run test:runtimenpm --prefix .agent run check:workflownpm --prefix .agent run check:docsgit diff --checkCloses #496